Skip to content
This repository was archived by the owner on Jan 19, 2019. It is now read-only.

New: Add method decorators to AST (fixes #65) #64

Merged
merged 1 commit into from
Aug 24, 2016
Merged

New: Add method decorators to AST (fixes #65) #64

merged 1 commit into from
Aug 24, 2016

Conversation

JamesHenry
Copy link
Member

@JamesHenry JamesHenry commented Aug 22, 2016 •
edited
Loading

Closes #65

(Ignore the branch name! This PR relates to method decorators)

@eslintbot
Copy link

Thanks for the pull request, @JamesHenry! I took a look to make sure it's ready for merging and found some changes are needed:

  • Pull requests with code require an issue to be mentioned at the end of the commit summary, such as (fixes #1234). Please update the commit summary with an issue (file a new issue if one doesn't already exist).

Can you please update the pull request to address these?

(More information can be found in our pull request guide.)

@JamesHenry JamesHenry mentioned this pull request Aug 22, 2016
Closed
14 tasks
@JamesHenry
Copy link
Member Author

All in good time, my dear @eslintbot

@eslintbot
Copy link

Thanks for the pull request, @JamesHenry! I took a look to make sure it's ready for merging and found some changes are needed:

  • Pull requests with code require an issue to be mentioned at the end of the commit summary, such as (fixes #1234). Please update the commit summary with an issue (file a new issue if one doesn't already exist).

Can you please update the pull request to address these?

(More information can be found in our pull request guide.)

nzakas
nzakas reviewed Aug 22, 2016
View reviewed changes
lib/ast-converter.js
var exp = convertChild(decorator.expression);
return {
type: "Decorator",
loc: exp.loc,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on Flow, we don't need this extra node.

@eslintbot
Copy link

Thanks for the pull request, @JamesHenry! I took a look to make sure it's ready for merging and found some changes are needed:

  • Pull requests with code require an issue to be mentioned at the end of the commit summary, such as (fixes #1234). Please update the commit summary with an issue (file a new issue if one doesn't already exist).

Can you please update the pull request to address these?

(More information can be found in our pull request guide.)

@eslintbot
Copy link

LGTM

@JamesHenry JamesHenry changed the title New: Process decorators on MethodDefinition node New: Add method decorators to AST (fixes #65) Aug 24, 2016
@eslintbot
Copy link

LGTM

@eslintbot
Copy link

LGTM

@JamesHenry
Copy link
Member Author

@nzakas Now ready for review, PTAL

@nzakas
Copy link
Member

nzakas commented Aug 24, 2016

This looks great, nice work!

@nzakas nzakas merged commit 7364cb9 into eslint:master Aug 24, 2016
@JamesHenry JamesHenry deleted the parameter-decorator-instance-member branch August 24, 2016 23:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA: Valid
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@JamesHenry @eslintbot @nzakas @jquerybot